[clang-format]: Add StaticInlineOnly and StaticInline options to ShortFunctionStyle
#133598
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…`ShortFunctionStyle`
Currently, the `ShortFunctionStyle` option in clang-format lacks the granularity to specifically control the single-line formatting of `static inline` C functions independently from other function types like regular empty functions.
**Problem:**
Users may want to enforce a style where:
1. **Only `static inline` functions** are allowed on a single line (if they fit), forcing all other functions (including empty ones) onto multiple lines. This is useful for keeping utility/helper functions concise while maintaining a consistent multi-line format for primary function definitions.
2. **`static inline` functions *or* empty functions** are allowed on a single line (if they fit), while other non-empty, non-`static inline` functions are forced onto multiple lines. This is a slightly less strict variation.
The existing `ShortFunctionStyle` options do not cover these specific C use cases adequately:
* `None`: Forces all functions multi-line.
* `Empty`: Allows *any* empty function on one line, not just `static inline` ones.
* `All`: Allows any short function on one line.
* `Inline`/`InlineOnly`: Primarily target C++ member functions or C++ free inline functions, not specifically the C `static inline` pattern.
**Proposed Solution:**
Introduce two new values for the `ShortFunctionStyle` enum (currently named `ShortFunctionStyle` internally, likely `SFS_...` values):
1. **`StaticInlineOnly`**
* **Configuration Name:** `StaticInlineOnly`
* **Internal Enum Value (Suggestion):** `SFS_StaticInlineOnly`
* **Behavior:** Allows *only* functions declared with both `static` and `inline` specifiers to be formatted on a single line, provided they fit within the `ColumnLimit`. All other functions (regular, static non-inline, inline non-static, empty or not) must be formatted across multiple lines.
2. **`StaticInline`**
* **Configuration Name:** `StaticInline`
* **Internal Enum Value (Suggestion):** `SFS_StaticInline`
* **Behavior:** Allows functions declared with both `static` and `inline` specifiers *or* functions with an empty body (`{}`) to be formatted on a single line, provided they fit within the `ColumnLimit`. Non-empty functions that are *not* `static inline` must be formatted across multiple lines. This effectively combines the `SFS_Empty` behavior with allowing non-empty `static inline` functions.
**Expected Formatting:**
* **With `ShortFunctionStyle: StaticInlineOnly`**
```c
void f1(void) // Multi-line (not static inline)
{
}
int f2(int a, int b) // Multi-line (not static inline)
{
return a + b;
}
static void f3(void) // Multi-line (not static inline)
{
}
static int f4(int a, int b) // Multi-line (not static inline)
{
return a + b;
}
static inline void f5(void) {} // Single-line allowed
static inline int f6(int a, int b) { return a + b; } // Single-line allowed (if fits)
inline void f7(void) // Multi-line (not static inline)
{
}
```
* **With `ShortFunctionStyle: StaticInline`** (Implies Empty)
```c
void f1(void) {} // Single-line allowed (empty)
int f2(int a, int b) // Multi-line (non-empty, not static inline)
{
return a + b;
}
static void f3(void) {} // Single-line allowed (empty)
static int f4(int a, int b) // Multi-line (non-empty, not static inline)
{
return a + b;
}
static inline void f5(void) {} // Single-line allowed (static inline and empty)
static inline int f6(int a, int b) { return a + b; } // Single-line allowed (static inline, if fits)
inline void f7(void) {} // Single-line allowed (empty)
```
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: Ivan (irymarchyk) ChangesCurrently, the Problem: Users may want to enforce a style where:
The existing
Proposed Solution: Introduce two new values for the
Full diff: https://github.com/llvm/llvm-project/pull/133598.diff 7 Files Affected:
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 9ecac68ae72bf..e5641fa5037ae 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -1926,6 +1926,15 @@ the configuration (without a prefix: ``Auto``).
void f() {
}
+ * ``SFS_StaticInlineOnly`` (in configuration: ``StaticInlineOnly``)
+ Only merge functions defined as static inline.
+
+ .. code-block:: c++
+
+ void f5(void) {
+ }
+ static inline int f6(int a, int b) { return a + b; }
+
* ``SFS_Empty`` (in configuration: ``Empty``)
Only merge empty functions.
@@ -1949,6 +1958,14 @@ the configuration (without a prefix: ``Auto``).
}
void f() {}
+ * ``SFS_StaticInline`` (in configuration: ``StaticInline``)
+ Only merge functions defined as static inline. Implies ``empty``.
+
+ .. code-block:: c++
+
+ void f5(void) {}
+ static inline int f6(int a, int b) { return a + b; }
+
* ``SFS_All`` (in configuration: ``All``)
Merge all functions fitting on a single line.
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index fec47a248abb4..7fa9f87422867 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -844,6 +844,13 @@ struct FormatStyle {
/// }
/// \endcode
SFS_InlineOnly,
+ /// Only merge functions defined as static inline.
+ /// \code
+ /// void f5(void) {
+ /// }
+ /// static inline int f6(int a, int b) { return a + b; }
+ /// \endcode
+ SFS_StaticInlineOnly,
/// Only merge empty functions.
/// \code
/// void f() {}
@@ -863,6 +870,12 @@ struct FormatStyle {
/// void f() {}
/// \endcode
SFS_Inline,
+ /// Only merge functions defined as static inline. Implies ``empty``.
+ /// \code
+ /// void f5(void) {}
+ /// static inline int f6(int a, int b) { return a + b; }
+ /// \endcode
+ SFS_StaticInline,
/// Merge all functions fitting on a single line.
/// \code
/// class Foo {
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 28aea86139e0d..05ed4f4a2bec8 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -622,6 +622,8 @@ template <> struct ScalarEnumerationTraits<FormatStyle::ShortFunctionStyle> {
IO.enumCase(Value, "Inline", FormatStyle::SFS_Inline);
IO.enumCase(Value, "InlineOnly", FormatStyle::SFS_InlineOnly);
IO.enumCase(Value, "Empty", FormatStyle::SFS_Empty);
+ IO.enumCase(Value, "StaticInlineOnly", FormatStyle::SFS_StaticInlineOnly);
+ IO.enumCase(Value, "StaticInline", FormatStyle::SFS_StaticInline);
}
};
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index d87b3a6088bd8..2b2177c542d88 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5690,8 +5690,10 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
return Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_None ||
Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Empty ||
(Left.NestingLevel == 0 && Line.Level == 0 &&
- Style.AllowShortFunctionsOnASingleLine &
- FormatStyle::SFS_InlineOnly);
+ (Style.AllowShortFunctionsOnASingleLine ==
+ FormatStyle::SFS_InlineOnly ||
+ Style.AllowShortFunctionsOnASingleLine ==
+ FormatStyle::SFS_Inline));
}
} else if (Style.Language == FormatStyle::LK_Java) {
if (Right.is(tok::plus) && Left.is(tok::string_literal) && AfterRight &&
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 000a5105ca407..ceca9c36fff7b 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -300,8 +300,9 @@ class LineJoiner {
return true;
}
- if (Style.AllowShortFunctionsOnASingleLine &
- FormatStyle::SFS_InlineOnly) {
+ if (Style.AllowShortFunctionsOnASingleLine ==
+ FormatStyle::SFS_InlineOnly ||
+ Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline) {
// Just checking TheLine->Level != 0 is not enough, because it
// provokes treating functions inside indented namespaces as short.
if (Style.isJavaScript() && TheLine->Last->is(TT_FunctionLBrace))
@@ -335,6 +336,32 @@ class LineJoiner {
}
}
+ if (Style.AllowShortFunctionsOnASingleLine ==
+ FormatStyle::SFS_StaticInlineOnly ||
+ Style.AllowShortFunctionsOnASingleLine ==
+ FormatStyle::SFS_StaticInline) {
+ // Check if the current line belongs to a static inline function
+ const auto *FirstNonCommentToken =
+ TheLine ? TheLine->getFirstNonComment() : nullptr;
+
+ // Look for 'static' and 'inline' keywords in any order
+ bool HasStatic = false;
+ bool HasInline = false;
+ const FormatToken *Tok = FirstNonCommentToken;
+
+ while (Tok && !Tok->is(TT_FunctionLBrace)) {
+ if (Tok->is(tok::kw_static))
+ HasStatic = true;
+ if (Tok->is(tok::kw_inline))
+ HasInline = true;
+ Tok = Tok->Next;
+ }
+
+ // If we found both static and inline, allow merging
+ if (HasStatic && HasInline)
+ return true;
+ }
+
return false;
};
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 287191d04d885..d7dfe577bedae 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -620,6 +620,12 @@ TEST(ConfigParseTest, ParsesConfiguration) {
AllowShortFunctionsOnASingleLine, FormatStyle::SFS_Empty);
CHECK_PARSE("AllowShortFunctionsOnASingleLine: All",
AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
+ CHECK_PARSE("AllowShortFunctionsOnASingleLine: StaticInlineOnly",
+ AllowShortFunctionsOnASingleLine,
+ FormatStyle::SFS_StaticInlineOnly);
+ CHECK_PARSE("AllowShortFunctionsOnASingleLine: StaticInline",
+ AllowShortFunctionsOnASingleLine, FormatStyle::SFS_StaticInline);
+
// For backward compatibility:
CHECK_PARSE("AllowShortFunctionsOnASingleLine: false",
AllowShortFunctionsOnASingleLine, FormatStyle::SFS_None);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 0b90bd360b758..2fec608055ac9 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -15120,6 +15120,85 @@ TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
MergeInlineOnly);
}
+TEST_F(FormatTest, PullStaticInlineFunctionDefinitionsIntoSingleLine) {
+ FormatStyle MergeStaticInlineOnly = getLLVMStyle();
+ MergeStaticInlineOnly.AllowShortFunctionsOnASingleLine =
+ FormatStyle::SFS_StaticInlineOnly;
+ verifyFormat("static inline int f() { return 42; }",
+ "static inline int f() {\n"
+ " return 42; \n"
+ "}",
+ MergeStaticInlineOnly);
+ verifyFormat("inline static int f() { return 42; }",
+ "inline static int f() \n"
+ "{\n"
+ " return 42; \n"
+ "}",
+ MergeStaticInlineOnly);
+ verifyNoChange("int f() {\n"
+ " return 42;\n"
+ "}",
+ MergeStaticInlineOnly);
+
+ verifyFormat("void f1(void) {\n"
+ "}",
+ "void f1(void)\n"
+ "{\n"
+ "}",
+ MergeStaticInlineOnly);
+
+ verifyNoChange("int f2(int a, int b) {\n"
+ " return a + b;\n"
+ "}",
+ MergeStaticInlineOnly);
+
+ verifyNoChange("static void f3(void) {\n"
+ "}\n",
+ MergeStaticInlineOnly);
+
+ verifyFormat("static int f4(int a, int b) {\n"
+ " return a + b;\n"
+ "}\n",
+ MergeStaticInlineOnly);
+
+ verifyNoChange("static inline void f5(void) {}", MergeStaticInlineOnly);
+
+ verifyFormat("static inline int f6(int a, int b) { return a + b; }",
+ "static inline int f6(int a, int b) \n"
+ "{ return a + b; }",
+ MergeStaticInlineOnly);
+
+ verifyFormat("int f(int a, int b) {\n"
+ " return a + b;\n"
+ "}",
+ "int f(int a, int b) { return a + b; }", MergeStaticInlineOnly);
+
+ FormatStyle MergeStaticInline = getLLVMStyle();
+ MergeStaticInline.AllowShortFunctionsOnASingleLine =
+ FormatStyle::SFS_StaticInline;
+ verifyFormat("static inline int f() { return 42; }",
+ "static inline int f() {\n"
+ " return 42; \n"
+ "}",
+ MergeStaticInline);
+ verifyFormat("inline static int f() { return 42; }",
+ "inline static int f() \n"
+ "{\n"
+ " return 42; \n"
+ "}",
+ MergeStaticInline);
+ verifyNoChange("int f() {\n"
+ " return 42;\n"
+ "}",
+ MergeStaticInline);
+
+ verifyFormat("void f1(void) {}",
+ "void f1(void)\n"
+ "{\n"
+ "}",
+ MergeStaticInline);
+}
+
TEST_F(FormatTest, PullInlineOnlyFunctionDefinitionsIntoSingleLine) {
FormatStyle MergeInlineOnly = getLLVMStyle();
MergeInlineOnly.AllowShortFunctionsOnASingleLine =
|
Member
|
@llvm/pr-subscribers-clang-format Author: Ivan (irymarchyk) ChangesCurrently, the Problem: Users may want to enforce a style where:
The existing
Proposed Solution: Introduce two new values for the
Full diff: https://github.com/llvm/llvm-project/pull/133598.diff 7 Files Affected:
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 9ecac68ae72bf..e5641fa5037ae 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -1926,6 +1926,15 @@ the configuration (without a prefix: ``Auto``).
void f() {
}
+ * ``SFS_StaticInlineOnly`` (in configuration: ``StaticInlineOnly``)
+ Only merge functions defined as static inline.
+
+ .. code-block:: c++
+
+ void f5(void) {
+ }
+ static inline int f6(int a, int b) { return a + b; }
+
* ``SFS_Empty`` (in configuration: ``Empty``)
Only merge empty functions.
@@ -1949,6 +1958,14 @@ the configuration (without a prefix: ``Auto``).
}
void f() {}
+ * ``SFS_StaticInline`` (in configuration: ``StaticInline``)
+ Only merge functions defined as static inline. Implies ``empty``.
+
+ .. code-block:: c++
+
+ void f5(void) {}
+ static inline int f6(int a, int b) { return a + b; }
+
* ``SFS_All`` (in configuration: ``All``)
Merge all functions fitting on a single line.
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index fec47a248abb4..7fa9f87422867 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -844,6 +844,13 @@ struct FormatStyle {
/// }
/// \endcode
SFS_InlineOnly,
+ /// Only merge functions defined as static inline.
+ /// \code
+ /// void f5(void) {
+ /// }
+ /// static inline int f6(int a, int b) { return a + b; }
+ /// \endcode
+ SFS_StaticInlineOnly,
/// Only merge empty functions.
/// \code
/// void f() {}
@@ -863,6 +870,12 @@ struct FormatStyle {
/// void f() {}
/// \endcode
SFS_Inline,
+ /// Only merge functions defined as static inline. Implies ``empty``.
+ /// \code
+ /// void f5(void) {}
+ /// static inline int f6(int a, int b) { return a + b; }
+ /// \endcode
+ SFS_StaticInline,
/// Merge all functions fitting on a single line.
/// \code
/// class Foo {
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 28aea86139e0d..05ed4f4a2bec8 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -622,6 +622,8 @@ template <> struct ScalarEnumerationTraits<FormatStyle::ShortFunctionStyle> {
IO.enumCase(Value, "Inline", FormatStyle::SFS_Inline);
IO.enumCase(Value, "InlineOnly", FormatStyle::SFS_InlineOnly);
IO.enumCase(Value, "Empty", FormatStyle::SFS_Empty);
+ IO.enumCase(Value, "StaticInlineOnly", FormatStyle::SFS_StaticInlineOnly);
+ IO.enumCase(Value, "StaticInline", FormatStyle::SFS_StaticInline);
}
};
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index d87b3a6088bd8..2b2177c542d88 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5690,8 +5690,10 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
return Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_None ||
Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Empty ||
(Left.NestingLevel == 0 && Line.Level == 0 &&
- Style.AllowShortFunctionsOnASingleLine &
- FormatStyle::SFS_InlineOnly);
+ (Style.AllowShortFunctionsOnASingleLine ==
+ FormatStyle::SFS_InlineOnly ||
+ Style.AllowShortFunctionsOnASingleLine ==
+ FormatStyle::SFS_Inline));
}
} else if (Style.Language == FormatStyle::LK_Java) {
if (Right.is(tok::plus) && Left.is(tok::string_literal) && AfterRight &&
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 000a5105ca407..ceca9c36fff7b 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -300,8 +300,9 @@ class LineJoiner {
return true;
}
- if (Style.AllowShortFunctionsOnASingleLine &
- FormatStyle::SFS_InlineOnly) {
+ if (Style.AllowShortFunctionsOnASingleLine ==
+ FormatStyle::SFS_InlineOnly ||
+ Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline) {
// Just checking TheLine->Level != 0 is not enough, because it
// provokes treating functions inside indented namespaces as short.
if (Style.isJavaScript() && TheLine->Last->is(TT_FunctionLBrace))
@@ -335,6 +336,32 @@ class LineJoiner {
}
}
+ if (Style.AllowShortFunctionsOnASingleLine ==
+ FormatStyle::SFS_StaticInlineOnly ||
+ Style.AllowShortFunctionsOnASingleLine ==
+ FormatStyle::SFS_StaticInline) {
+ // Check if the current line belongs to a static inline function
+ const auto *FirstNonCommentToken =
+ TheLine ? TheLine->getFirstNonComment() : nullptr;
+
+ // Look for 'static' and 'inline' keywords in any order
+ bool HasStatic = false;
+ bool HasInline = false;
+ const FormatToken *Tok = FirstNonCommentToken;
+
+ while (Tok && !Tok->is(TT_FunctionLBrace)) {
+ if (Tok->is(tok::kw_static))
+ HasStatic = true;
+ if (Tok->is(tok::kw_inline))
+ HasInline = true;
+ Tok = Tok->Next;
+ }
+
+ // If we found both static and inline, allow merging
+ if (HasStatic && HasInline)
+ return true;
+ }
+
return false;
};
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 287191d04d885..d7dfe577bedae 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -620,6 +620,12 @@ TEST(ConfigParseTest, ParsesConfiguration) {
AllowShortFunctionsOnASingleLine, FormatStyle::SFS_Empty);
CHECK_PARSE("AllowShortFunctionsOnASingleLine: All",
AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
+ CHECK_PARSE("AllowShortFunctionsOnASingleLine: StaticInlineOnly",
+ AllowShortFunctionsOnASingleLine,
+ FormatStyle::SFS_StaticInlineOnly);
+ CHECK_PARSE("AllowShortFunctionsOnASingleLine: StaticInline",
+ AllowShortFunctionsOnASingleLine, FormatStyle::SFS_StaticInline);
+
// For backward compatibility:
CHECK_PARSE("AllowShortFunctionsOnASingleLine: false",
AllowShortFunctionsOnASingleLine, FormatStyle::SFS_None);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 0b90bd360b758..2fec608055ac9 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -15120,6 +15120,85 @@ TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
MergeInlineOnly);
}
+TEST_F(FormatTest, PullStaticInlineFunctionDefinitionsIntoSingleLine) {
+ FormatStyle MergeStaticInlineOnly = getLLVMStyle();
+ MergeStaticInlineOnly.AllowShortFunctionsOnASingleLine =
+ FormatStyle::SFS_StaticInlineOnly;
+ verifyFormat("static inline int f() { return 42; }",
+ "static inline int f() {\n"
+ " return 42; \n"
+ "}",
+ MergeStaticInlineOnly);
+ verifyFormat("inline static int f() { return 42; }",
+ "inline static int f() \n"
+ "{\n"
+ " return 42; \n"
+ "}",
+ MergeStaticInlineOnly);
+ verifyNoChange("int f() {\n"
+ " return 42;\n"
+ "}",
+ MergeStaticInlineOnly);
+
+ verifyFormat("void f1(void) {\n"
+ "}",
+ "void f1(void)\n"
+ "{\n"
+ "}",
+ MergeStaticInlineOnly);
+
+ verifyNoChange("int f2(int a, int b) {\n"
+ " return a + b;\n"
+ "}",
+ MergeStaticInlineOnly);
+
+ verifyNoChange("static void f3(void) {\n"
+ "}\n",
+ MergeStaticInlineOnly);
+
+ verifyFormat("static int f4(int a, int b) {\n"
+ " return a + b;\n"
+ "}\n",
+ MergeStaticInlineOnly);
+
+ verifyNoChange("static inline void f5(void) {}", MergeStaticInlineOnly);
+
+ verifyFormat("static inline int f6(int a, int b) { return a + b; }",
+ "static inline int f6(int a, int b) \n"
+ "{ return a + b; }",
+ MergeStaticInlineOnly);
+
+ verifyFormat("int f(int a, int b) {\n"
+ " return a + b;\n"
+ "}",
+ "int f(int a, int b) { return a + b; }", MergeStaticInlineOnly);
+
+ FormatStyle MergeStaticInline = getLLVMStyle();
+ MergeStaticInline.AllowShortFunctionsOnASingleLine =
+ FormatStyle::SFS_StaticInline;
+ verifyFormat("static inline int f() { return 42; }",
+ "static inline int f() {\n"
+ " return 42; \n"
+ "}",
+ MergeStaticInline);
+ verifyFormat("inline static int f() { return 42; }",
+ "inline static int f() \n"
+ "{\n"
+ " return 42; \n"
+ "}",
+ MergeStaticInline);
+ verifyNoChange("int f() {\n"
+ " return 42;\n"
+ "}",
+ MergeStaticInline);
+
+ verifyFormat("void f1(void) {}",
+ "void f1(void)\n"
+ "{\n"
+ "}",
+ MergeStaticInline);
+}
+
TEST_F(FormatTest, PullInlineOnlyFunctionDefinitionsIntoSingleLine) {
FormatStyle MergeInlineOnly = getLLVMStyle();
MergeInlineOnly.AllowShortFunctionsOnASingleLine =
|
clang/include/clang/Format/Format.h
Outdated
| /// void f() {} | ||
| /// \endcode | ||
| SFS_Inline, | ||
| /// Only merge functions defined as static inline. Implies ``empty``. |
Contributor
There was a problem hiding this comment.
This should have a different text.
Author
There was a problem hiding this comment.
Thanks, fixed. Now it mention that empty function also merged.
| const auto *FirstNonCommentToken = | ||
| TheLine ? TheLine->getFirstNonComment() : nullptr; | ||
|
|
||
| // Look for 'static' and 'inline' keywords in any order |
Contributor
There was a problem hiding this comment.
Suggested change
| // Look for 'static' and 'inline' keywords in any order | |
| // Look for 'static' and 'inline' keywords in any order. |
| bool HasInline = false; | ||
| const FormatToken *Tok = FirstNonCommentToken; | ||
|
|
||
| while (Tok && !Tok->is(TT_FunctionLBrace)) { |
Contributor
There was a problem hiding this comment.
You really don't need to go until the brace, the paren would suffice, or even better the name?
Author
There was a problem hiding this comment.
Seems like the name is sufficient. Thanks for idea. I've made a fix; please take a look.
| Tok = Tok->Next; | ||
| } | ||
|
|
||
| // If we found both static and inline, allow merging |
Contributor
There was a problem hiding this comment.
Suggested change
| // If we found both static and inline, allow merging | |
| // If we found both static and inline, allow merging. |
| } | ||
|
|
||
| // If we found both static and inline, allow merging | ||
| if (HasStatic && HasInline) |
Contributor
There was a problem hiding this comment.
Move this check into the ifs of the loop, exit early.
Author
There was a problem hiding this comment.
Added additional check in while(). Please take a look.
| "{\n" | ||
| "}", | ||
| MergeStaticInline); | ||
| } |
Contributor
There was a problem hiding this comment.
Please add some tests with attributes.
Author
There was a problem hiding this comment.
I've added some tests with different attributes in different places. If you have any idea what should be added - small example (just code line) would be appreciated.
| "{\n" | ||
| "}", | ||
| MergeStaticInline); | ||
| } |
Contributor
There was a problem hiding this comment.
How about constexpr this implies inline, what should be done in this case?
Author
There was a problem hiding this comment.
I have no preference regarding constexpr because it was not mentioned in 'Inline' or 'InlineOnly' options before. Right now it won't be formatted if StaticInline is specified (I added unit test to show this). Please let me know if I need to change this.
…ons to ShortFunctionStyle * fixed small PR requests such as grammar + added additional unit tests to test with attribute in different places
irymarchyk
force-pushed
the
add_static_inline_support_for_short_line
branch
from
4522f85 to
3ccdaeb
Compare
Contributor
|
If @mydeveloperday doesn't object to adding more choices to |
Author
|
@owenca, just to confirm - you are suggesting to add |
Contributor
Yep.
Yes, but can you split it into two patches? The first would be NFC and only cover |
Contributor
|
I like what @owenca suggested |
Author
|
Thanks, I've added first changes here: #134337 . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the
ShortFunctionStyleoption in clang-format lacks the granularity to specifically control the single-line formatting ofstatic inlineC functions independently from other function types like regular empty functions.Problem:
Users may want to enforce a style where:
static inlinefunctions are allowed on a single line (if they fit), forcing all other functions (including empty ones) onto multiple lines. This is useful for keeping utility/helper functions concise while maintaining a consistent multi-line format for primary function definitions.static inlinefunctions or empty functions are allowed on a single line (if they fit), while other non-empty, non-static inlinefunctions are forced onto multiple lines. This is a slightly less strict variation.The existing
ShortFunctionStyleoptions do not cover these specific C use cases adequately:None: Forces all functions multi-line.Empty: Allows any empty function on one line, not juststatic inlineones.All: Allows any short function on one line.Inline/InlineOnly: Primarily target C++ member functions or C++ free inline functions, not specifically the Cstatic inlinepattern.Proposed Solution:
Introduce two new values for the
ShortFunctionStyleenum (currently namedShortFunctionStyleinternally, likelySFS_...values):StaticInlineOnlyStaticInlineOnlySFS_StaticInlineOnlystaticandinlinespecifiers to be formatted on a single line, provided they fit within theColumnLimit. All other functions (regular, static non-inline, inline non-static, empty or not) must be formatted across multiple lines.StaticInlineStaticInlineSFS_StaticInlinestaticandinlinespecifiers or functions with an empty body ({}) to be formatted on a single line, provided they fit within theColumnLimit. Non-empty functions that are notstatic inlinemust be formatted across multiple lines. This effectively combines theSFS_Emptybehavior with allowing non-emptystatic inlinefunctions.Expected Formatting:
With
ShortFunctionStyle: StaticInlineOnlyWith
ShortFunctionStyle: StaticInline(Implies Empty)